Skip to content

Conversation

@nasahlpa
Copy link
Member

@nasahlpa nasahlpa commented Nov 5, 2025

This PR fixes two different test failures:

  • Remove leading 0 for the modulo
  • Allow longer signature sizes

Those failures are not related to bugs in the cryptolib but in the cryptotest framework.

@nasahlpa nasahlpa requested review from a team as code owners November 5, 2025 15:10
@nasahlpa nasahlpa requested review from timothytrippel and removed request for a team November 5, 2025 15:10
Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this, @nasahlpa.

When I apply the changes from my comments, the failing test vectors from before are now passing, but I'm seeing a new failure in (at least) the first test vector of sw/host/cryptotest/testvectors/data/wycheproof_rsa_pss_3072_shake128.json. I see that this is documented in #28656 - is this intended to be fixed in a separate PR?

The wycheproof testvector seem to have a leading 0. This means
that a 512-byte modulo for RSA-4096 is actually 513-byte.
Currently, the test framework fails because it expects a 512-byte
value. Remove this leading 0.

Signed-off-by: Pascal Nasahl <[email protected]>
Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, probably needs to be backported to earlgrey_1.0.0 as well?

Is the intention to fix the remaining RSA test failures in #28656 (and thus the failures in //sw/device/tests/crypto/cryptotest:rsa_kat) in separate PRs?

@nasahlpa
Copy link
Member Author

nasahlpa commented Nov 5, 2025

Thanks for the review, Alex!

Yes, the intention is to fix this in upcoming PRs and in the meanwhile track the failing tests in #28656.

@nasahlpa nasahlpa added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Nov 5, 2025
Some tests in the wycheproof test vector set are error tests. In one
of these error tests, a larger signature (514 instead of 512-bytes) is
sent to the DUT. However, we currently limit the signature payload to 512-
bytes, letting the test fail.

The solution to this is simple, increase the max. number of signature bytes
to 512-bytes. The CL now gets this 514-byte signature and internally fails,
as expected by the test.

Signed-off-by: Pascal Nasahl <[email protected]>
@andrea-caforio
Copy link
Contributor

Which tests does this fix?

@nasahlpa
Copy link
Member Author

nasahlpa commented Nov 6, 2025

Which tests does this fix?

For the first commit:
The RSA-4096 verify tests. The reason is that we only allow a max. 512-bytes of modulos:


However, for RSA-4096, n is actually 513-bytes because of the leading 0.

For the second commit:
Again for RSA-4096 verify. In some error cases that are expected to fail is the signature 513 or 514 bytes - then the Rust framework failed because it allowed a max. signature size of only 512 bytes.

Copy link
Contributor

@andrea-caforio andrea-caforio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Thanks @nasahlpa.

@nasahlpa nasahlpa added this pull request to the merge queue Nov 6, 2025
Merged via the queue into lowRISC:master with commit 250cfee Nov 6, 2025
47 checks passed
@lowrisc-ci
Copy link

lowrisc-ci bot commented Nov 6, 2025

Successfully created backport PR for earlgrey_1.0.0:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants